Skip to content

fix(getRange): nextStartPrimaryKey type incorrect#9

Merged
BlackHole1 merged 1 commit intomainfrom
fix-type
Nov 8, 2025
Merged

fix(getRange): nextStartPrimaryKey type incorrect#9
BlackHole1 merged 1 commit intomainfrom
fix-type

Conversation

@BlackHole1
Copy link
Member

No description provided.

Signed-off-by: Kevin Cui <bh@bugs.cc>
Copilot AI review requested due to automatic review settings November 8, 2025 02:06
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 8, 2025

Summary by CodeRabbit

  • Refactor
    • Updated the response structure for range query operations. The nextStartPrimaryKey field now returns an array of rows instead of a single row. Response initialization and parsing logic have been adjusted to reflect this structural change, ensuring compatibility with the new array-based return type for pagination operations.

Walkthrough

The pull request modifies the GetRangeResponse.nextStartPrimaryKey property in src/operator/get-range.ts, changing its type from PlainBufferRow | null to Array<PlainBufferRow> | null. This alters the expected shape of the next-start primary key from a single row object to an array of row objects. The corresponding initialization and response parsing logic have been updated to handle the new array-based structure.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Limited scope: Single file modification with localized changes
  • Straightforward type transformation with consistent pattern updates
  • Changes follow a clear structural pattern (type definition → initialization → response parsing)

Possibly related PRs

Pre-merge checks

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess relevance to the changeset. Add a pull request description explaining the type change from PlainBufferRow to Array and the rationale behind this modification.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title follows the required format '(): ' with 'fix' as type, 'getRange' as scope, and clearly describes the main change of correcting the nextStartPrimaryKey type.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c1ebe60 and fea2659.

📒 Files selected for processing (1)
  • src/operator/get-range.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/operator/get-range.ts (2)
src/index.ts (1)
  • PlainBufferRow (109-109)
src/plainbuffer.ts (1)
  • PlainBufferRow (652-652)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Upload results
🔇 Additional comments (2)
src/operator/get-range.ts (2)

108-111: Implementation already returned an array - type mismatch existed.

The implementation has always returned Array<PlainBufferRow> via decodePlainBuffer(), identical to how rows is decoded (line 105). The previous type definition of PlainBufferRow | null was incorrect, creating a type-implementation mismatch.

This change fixes the type alignment but introduces a breaking change for consumers who were expecting a single row based on the previous type definition.


31-31: I need to search more broadly for how GetRangeResponse is consumed:

Now let me verify how decodePlainBuffer is actually defined and what PlainBufferRow represents:

Let me verify one more time by searching for actual usage patterns of nextStartPrimaryKey in similar TypeScript/Node.js codebases:

Based on my verification using official TableStore documentation and SDK samples, I can now provide the final assessment:


The type change is correct and requires no modifications. The official Node.js TableStore SDK shows nextStartPrimaryKey is accessed as an array (e.g., data.nextStartPrimaryKey[0].value, data.nextStartPrimaryKey[1].value), confirming that Array<PlainBufferRow> | null is the appropriate type. This change aligns the TypeScript SDK with the existing implementation and matches the standard pattern across TableStore SDKs. The original review comment's concerns about extracting the first element are unfounded—the array structure correctly represents the multi-column primary key used for pagination.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a type inconsistency in the GetRangeResponse interface by correcting the nextStartPrimaryKey field type from PlainBufferRow | null to Array<PlainBufferRow> | null.

  • Aligns the type definition with the actual return value from decodePlainBuffer(), which returns an array
  • Makes the type consistent with similar fields in other operator response interfaces (GetRow, PutRow, DeleteRow, UpdateRow)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@BlackHole1 BlackHole1 merged commit 8f0d3d3 into main Nov 8, 2025
8 checks passed
@BlackHole1 BlackHole1 deleted the fix-type branch November 8, 2025 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants